Skip to content

Add a periodic job for building and test musl #849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add a periodic job for building and test musl #849

wants to merge 9 commits into from

Conversation

jsturtevant
Copy link
Contributor

@jsturtevant jsturtevant commented Sep 2, 2025 •
edited
Loading

fixes: #606

This pull request refactors the CI workflows and Justfile to improve support for musl target. It refactors the rust build job so it can be re-used across multiple targets and other github workflows. This doesn't currently reduce the number of jobs we run on PR's but sets the foundation for being able to reduce the number of items run.

When enabling musl I found a few items which will need follow up (will create issues):

  • Building tracy-client (used in one of the tests) didn't build on musl cleanly. I was able to get it build using cross but there was more extensive changes require to get cross working so disabled it for now, the rest of the other examples work. updated to use cross-rs
  • The seccomp tests fail with signal: 11, SIGSEGV: invalid memory reference when executing host functions that violate a seccomp rule. We've seen a few other issues around this code so to get basic coverage up I've disabled this for now and plan to revisit enabling this feature.
  • Includes a fix for CI instability on test_gdb_end_to_end  #626. Not sure why this was easier to reproduce with musl builds but I ran into this race condition a couple times while testing.

This does require installing some tools on the host VM's so I've marked this in draft until those tools are enabled. Done

You can see an a run where I enabled this temporarily on Pull requests to make sure if runs: https://github.com/hyperlight-dev/hyperlight/actions/runs/17632377041

@jsturtevant jsturtevant added kind/dependencies For PRs that update dependencies or related components kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. and removed kind/dependencies For PRs that update dependencies or related components labels Sep 2, 2025
@jsturtevant jsturtevant force-pushed the build-musl branch 14 times, most recently from 56b3b5a to 9641c92 Compare September 10, 2025 23:25
@jsturtevant jsturtevant marked this pull request as ready for review September 11, 2025 02:45
@jsturtevant jsturtevant force-pushed the build-musl branch 2 times, most recently from f5daa4a to 1adb011 Compare September 11, 2025 03:52
simongdavies
simongdavies reviewed Sep 11, 2025
View reviewed changes
Copy link
Contributor

@simongdavies simongdavies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. A couple of thoughts:

Do we want to add a recipe to the Justfile similar to like-ci that also includes the nightly stuff.

Should we move some of the stuff we are doing on each PR to the nightly job (e.g. don't think we need both the Windowws Server versions on each PR)

@simongdavies
Copy link
Contributor

Was it expected that the code checks got skipped?

ludfjig
ludfjig reviewed Sep 11, 2025
View reviewed changes
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Should we run tests too, in addition the cargo check?

@jsturtevant
Copy link
Contributor Author

jsturtevant commented Sep 11, 2025 •
edited
Loading

Do we want to add a recipe to the Justfile similar to like-ci that also includes the nightly stuff.

The nice thing about the way this works now is that the just like-ci stays the same. For the musl you can run:

TARGET_TRIPLE=x86_64-unknown-linux-musl just like-ci debug "features"

or

can use https://just.systems/man/en/setting-variables-from-the-command-line.html

just target-triple=x86_64-unknown-linux-musl like-ci debug "features"

Should we move some of the stuff we are doing on each PR to the nightly job (e.g. don't think we need both the Windowws Server versions on each PR)

I was thinking we should do this, I didn't know if we wanted to do that here or as a separate PR.

Was it expected that the code checks got skipped?

Yes we weren't running the code checks for all platforms. When I extracted the logic out in the rust_dep.yaml I added a skip (if) to those so we didn't start running extra, which is why they show up as skips explicitly.

We can either:

  • pull the checks out to their own job and only run it stand alone for the platforms we want it run on
  • leave it as is with skips, or as we move things over we wouldn't need the skip (if) since we would run less on PR's and run the other platforms on the cron job

@jsturtevant
Refactor dep_rust so it is re-usable in new nightly job
fd12214 
Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant
Refactor CI failure and add to nightly job
059a896 
Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant
Filter the location to a local path relative to the project
ff140e3 
this helps in ci when using the musl container which will end up with the wrong path

Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant
fixup! Refactor CI failure and add to nightly job
443deaa 
Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant
fixup! Disable seccomp for musl target
112bbdc 
Signed-off-by: James Sturtevant <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simongdavies simongdavies simongdavies left review comments

@ludfjig ludfjig ludfjig left review comments

@danbugs danbugs Awaiting requested review from danbugs danbugs is a code owner

@dblnz dblnz Awaiting requested review from dblnz dblnz is a code owner

@devigned devigned Awaiting requested review from devigned devigned is a code owner

@syntactically syntactically Awaiting requested review from syntactically syntactically is a code owner

@marosset marosset Awaiting requested review from marosset marosset is a code owner

@jprendes jprendes Awaiting requested review from jprendes jprendes is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for x86_64-unknown-linux-musl target
3 participants
@jsturtevant @simongdavies @ludfjig